Skip to content

feat: cross-platform force-kill primitive for stuck PHP threads#2365

Open
nicolas-grekas wants to merge 1 commit intophp:mainfrom
nicolas-grekas:force-kill-primitives
Open

feat: cross-platform force-kill primitive for stuck PHP threads#2365
nicolas-grekas wants to merge 1 commit intophp:mainfrom
nicolas-grekas:force-kill-primitives

Conversation

@nicolas-grekas
Copy link
Copy Markdown
Contributor

@nicolas-grekas nicolas-grekas commented Apr 22, 2026

First step of the split suggested in #2287: land the force-kill
infrastructure as a standalone, reviewable primitive independent of
background workers. Useful on its own, and gives follow-up
graceful-shutdown work a reviewed foundation.

Design

Each PHP thread, at boot from its own TSRM context, hands a
force_kill_slot (pointers to its EG(vm_interrupt) and EG(timed_out)
atomic bools, plus pthread_t / Windows HANDLE) back to Go via
go_frankenphp_store_force_kill_slot. The slot lives on phpThread,
protected by a per-thread RWMutex so the zero-and-release path at
thread exit cannot race an in-flight kill. frankenphp_force_kill_thread
stores true into both atomic bools (waking the VM at the next opcode
boundary, routing through zend_timeout -> "Maximum execution time
exceeded"), then delivers a platform-specific wake-up:

  • Linux/FreeBSD: pthread_kill(SIGRTMIN+3) with a no-op handler
    installed once via pthread_once, SA_ONSTACK, no SA_RESTART.
    Signal delivery returns any in-flight blocking syscall with EINTR.
  • Windows: CancelSynchronousIo + QueueUserAPC covers alertable
    I/O and SleepEx. Non-alertable Sleep (including PHP's usleep)
    stays uninterruptible.
  • macOS: atomic-bool path only; threads stuck in blocking syscalls
    wait to return on their own.

Reserved signal: SIGRTMIN+3. A PHP script that calls
pcntl_signal(SIGRTMIN+3, ...) clobbers this. Embedders whose own Go
code uses SIGRTMIN+3 must patch it here. glibc NPTL reserves
SIGRTMIN..SIGRTMIN+2, so the offset cannot go lower.

JIT caveat: under OPcache JIT some hot paths skip the
vm_interrupt check (see php-src#21267).
A JIT busy loop may not observe the atomic-bool store and falls through
to the abandon path.

Drain flow

drainWorkerThreads waits drainGracePeriod (5s) for each thread to
reach Yielding, then arms force-kill on stragglers and waits
forceKillDeadline (5s) more. Threads still stuck past that are
abandoned rather than hanging the drain forever:

  • Abandoned threads are marked with the new state.Abandoned; handlers
    treat it like ShuttingDown on the next callback so a late-unwinding
    abandoned thread exits the serve loop instead of running a request
    with stale context.
  • They are detached from worker.threads immediately so
    isAtThreadLimit and the scaler see accurate capacity; the
    auto-scaling slice's cleanup drops Abandoned/Done entries too.
  • RestartWorkers now returns errIncompleteRestart wrapped with
    abandoned/restarted counts; admin endpoints and the watcher surface
    it.
  • phpThread.shutdown mirrors the same grace + force-kill + abandon
    pattern so Shutdown cannot hang on an uninterruptible call either.

Safe main-thread teardown after abandonment

An abandoned thread that unwinds after sapi_shutdown / tsrm_shutdown
would touch freed TSRM storage (ts_free_thread) and torn-down SAPI
(php_request_shutdown via zend_catch). After the main thread's Done
signal, php_main calls go_frankenphp_can_teardown, which does a
bounded wait (mainThreadShutdownDeadline, 5s) on the lingeringThreads
WaitGroup. On timeout, teardown is skipped entirely: PHP's SAPI/TSRM
state leaks until process exit - the safe outcome. To prevent silent
corruption on re-Init after a skipped teardown, initPHPThreads latches
a teardownSkipped flag and returns the new ErrTeardownSkipped
sentinel. Embedders that observe errIncompleteRestart and want to
continue serving should terminate the process rather than call Init
again.

Lifecycle hardening

  • phpThreads and thread_metrics are intentionally left allocated
    across Shutdown so abandoned threads' late callbacks index safely.
  • initPHPThreads blocks on lingeringThreads (incremented on every
    php_thread entry, decremented at the single goto exit: label) so
    the next Init cycle cannot reassign them out from under a lingering
    thread, then frees the previous thread_metrics before reallocating.
  • shutdown_in_progress atomic gates the unhealthy-thread respawn path
    off once Shutdown starts.
  • RequestSafeStateChange and phpThread.shutdown's fast-fail accept
    Abandoned so Shutdown cannot park on a Restarting -> Abandoned
    transition.

Testing

TestRestartWorkersForceKillsStuckThread drives the full path via a
marker file so RestartWorkers only arms once the worker is proven
parked in sleep(), then asserts bounded elapsed time and that the
post-sleep line never runs.

API note

RestartWorkers now returns error (wraps errIncompleteRestart).
Source-compatible - callers that ignored it still compile - but worth
flagging for embedders. New exported ErrTeardownSkipped sentinel.

Comment thread frankenphp.c Outdated
Comment thread frankenphp.c Outdated
Comment thread frankenphp.c Outdated
Comment thread frankenphp.c Outdated
Comment thread worker.go Outdated
Comment thread frankenphp.c Outdated
Comment thread frankenphp.c Outdated
Copy link
Copy Markdown
Member

@withinboredom withinboredom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. FYI: bugs like php/php-src#21267 mean that sometimes JIT just will never hit these vm breakpoints. So, be prepared for bug reports that aren't related to this change, but are due to JIT.

Comment thread worker_test.go
Comment thread worker.go Outdated
Comment thread frankenphp.c Outdated
Comment thread frankenphp.c
Comment thread frankenphp.c
@dunglas
Copy link
Copy Markdown
Member

dunglas commented Apr 24, 2026

Good work! Shouldn't this code be directly on php-src (TSRM)? It could be useful in other contexts than FrankenPHP.

@nicolas-grekas
Copy link
Copy Markdown
Contributor Author

@dunglas dunno in the future, but at the moment, this allows providing the capablity to all versions of PHP, without having to wait for an hypothetical merge in 8.6+.

Comment thread phpthread.go Outdated
@henderkes
Copy link
Copy Markdown
Contributor

@dunglas dunno in the future, but at the moment, this allows providing the capablity to all versions of PHP, without having to wait for an hypothetical merge in 8.6+.

The two don't exclude each other, it would actually help getting this upstreamed because it will open the doors to more calls being switched to alertable. Once that lands in master, we can backport it in FrankenPHP for 8.4+.

Introduces a self-contained primitive that unblocks a PHP thread stuck
in a blocking call (sleep, synchronous I/O, etc.) so the graceful drain
used by RestartWorkers, DrainWorkers, and Shutdown makes progress
instead of hanging for the duration of the block. The primitive is
useful on its own and gives follow-up graceful-shutdown work a reviewed
foundation to build on.

Design: each PHP thread, at boot from its own TSRM context, hands a
force_kill_slot (pointers to its own EG(vm_interrupt) and EG(timed_out)
atomic bools, plus pthread_t / Windows HANDLE for the wake-up) back to
Go via go_frankenphp_store_force_kill_slot. The slot lives on phpThread
and is protected by a per-thread RWMutex so the zero-and-release path
at thread exit cannot race an in-flight kill. From any goroutine, Go
passes the slot back to frankenphp_force_kill_thread, which stores true
into both bools (waking the VM at the next opcode boundary and routing
through zend_timeout -> "Maximum execution time exceeded") and then
delivers a platform-specific wake-up:

- Linux/FreeBSD: pthread_kill(SIGRTMIN+3) with a no-op handler installed
  via pthread_once, SA_ONSTACK, no SA_RESTART. Signal delivery causes
  any in-flight blocking syscall to return EINTR.
- Windows: CancelSynchronousIo + QueueUserAPC covers alertable I/O and
  SleepEx. Non-alertable Sleep (including PHP's usleep) stays
  uninterruptible.
- macOS: atomic-bool-only path. Threads stuck in blocking syscalls wait
  to return on their own.

JIT caveat: under the OPcache JIT some hot code paths skip vm_interrupt
checks (see php-src#21267), so a pure-PHP busy loop under JIT may not
observe the store and will fall through to the abandon path below.

Drain flow:

- worker.go: drainWorkerThreads waits drainGracePeriod (5s) for each
  drained thread to reach Yielding; then arms force-kill on stragglers
  and waits forceKillDeadline (5s) more. Threads still stuck past that
  are abandoned rather than hanging the drain forever.
- drainWorkerThreads returns (drained, abandoned). RestartWorkers puts
  drained threads back to Ready and abandoned ones into the new
  state.Abandoned (handlers treat it like ShuttingDown on next
  callback) so an abandoned thread that finally unwinds exits instead
  of re-entering the serve loop under stale request state. Abandoned
  threads are also filtered out of worker.threads immediately so
  isAtThreadLimit and the scaler see accurate capacity; the matching
  deactivateThreads cleanup drops Abandoned/Done entries from the
  auto-scaling slice so they do not permanently consume a global
  scaling slot. If any were abandoned, RestartWorkers returns
  errIncompleteRestart wrapped with abandoned/restarted counts -
  admin endpoint and watcher surface it.
- phpthread.go: phpThread.shutdown mirrors the same grace + force-kill
  + abandon pattern so Shutdown cannot hang on an uninterruptible
  blocking call either. RequestSafeStateChange and shutdown's fast-fail
  WaitFor both accept Abandoned so Shutdown racing a RestartWorkers
  that marks a thread Abandoned does not park on a transition that
  will never come.

Lifecycle hardening: Shutdown intentionally leaves phpThreads allocated
and thread_metrics alive - an abandoned thread that eventually unwinds
still calls through the SAPI and lifecycle callbacks which index those
structures. initPHPThreads blocks on a package-level sync.WaitGroup
(Add on every php_thread entry, Done on every exit path, routed
through a single goto exit: label in php_thread so future return paths
cannot silently leak an Add) so the next Init cycle cannot reassign
them out from under a lingering abandoned thread, then frees the
previous allocation inside frankenphp_init_thread_metrics before
allocating fresh. A dedicated C-side atomic (shutdown_in_progress,
toggled by frankenphp_set_shutdown_in_progress) is the signal the
unhealthy-thread restart path uses to refuse respawning past Shutdown.

- go_frankenphp_store_force_kill_slot / clear_force_kill_slot /
  on_thread_shutdown: take the per-thread write lock; clear runs before
  ts_free_thread on both healthy and unhealthy exit paths so the
  captured &EG() pointers are zeroed before their backing storage is
  freed.
- php_thread unblocks FRANKENPHP_KILL_SIGNAL with pthread_sigmask at
  startup so Go's runtime signal mask cannot silently drop deliveries.

Safe main-thread teardown after abandonment: an abandoned thread that
unwinds after sapi_shutdown / tsrm_shutdown would touch freed TSRM
storage (ts_free_thread) and torn-down SAPI (php_request_shutdown via
zend_catch, ub_write, etc). php_main now calls back into Go through
go_frankenphp_can_teardown after the main thread's Done signal; Go
does a bounded wait (mainThreadShutdownDeadline, 5s) on the
lingeringThreads WaitGroup. If every thread has exited, teardown runs
normally; on timeout Go logs a warning and returns false, and the C
side skips frankenphp_sapi_module.shutdown / sapi_shutdown /
tsrm_shutdown entirely. PHP's SAPI/TSRM state then leaks until process
exit, which is the safe outcome: a subsequent Shutdown after an
errIncompleteRestart can no longer crash on a late-unwinding abandoned
thread. Callers that want a fully-clean process after observing
errIncompleteRestart should terminate rather than re-Init, since a
skipped teardown leaves SAPI un-torn-down and the next Init would try
to initialise on top of it. initPHPThreads now enforces this
directly: it latches teardownSkipped when the bounded wait expires and
returns ErrTeardownSkipped on any subsequent Init, so embedders cannot
silently re-initialise on top of never-torn-down SAPI/TSRM.

- worker_test.go + testdata/worker-sleep.php: the regression test
  drives the full path via a request marker file so it only arms
  RestartWorkers once the worker is proven parked in sleep(), then
  asserts both the bounded elapsed time and that the "should not
  reach" line after sleep never runs (which would indicate the VM
  interrupt was never observed).

RestartWorkers now returns an error - a source-compatible Go API change
(callers that ignored it still compile) but worth noting for embedders.
@nicolas-grekas
Copy link
Copy Markdown
Contributor Author

nicolas-grekas commented Apr 24, 2026

PR should be ready! It took me a while to get it correct. PR description is updated. Lots of comments in the patch; let me know if that's too much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants